🚇 fix: Repair Direct Sandbox Startup And IPv6 Binding - #26
Conversation
|
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5290dbd91b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@LeonardSEO if you currently dont find the time could take over the PR and finish it as these would eliminate my custom fixes i have currently deployed. |
|
@snapydziuba Continued the PR and addressed the remaining review finding in |
|
Hi @LeonardSEO — thanks for splitting this into the parts that were fixable without touching the storage or egress architecture. That framing held up. Reviewing it against current Status of each part
#22 was fixed incidentally by #66, which replaced the whole bind sequence with a statically-linked #23 is untouched — #24's empty-mount hazard is untouched too, and #66 actually inherited it. if (access("/host-packages", F_OK) == 0 &&
bind_mount("/host-packages", "/pkgs", 0) != 0) {and What a rebase needsI test-merged this against The catch is that your conflicting block carries both the superseded static int has_entries(const char *path) {
DIR *dir = opendir(path);
if (dir == NULL) return 0;
struct dirent *entry;
int found = 0;
while ((entry = readdir(dir)) != NULL) {
if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) {
found = 1;
break;
}
}
closedir(dir);
return found;
}then gate the existing bind on
If it's easier, this is also fine to split: the IPv6 fix is self-contained, needs no conflict resolution at all, and could land on its own straight away. (Context: #22 is now closed as completed. This is the only open PR carrying fixes for #23 and #24, so I'd rather see it rebased than superseded.) |
Summary
This PR addresses the concrete runtime issues that can be fixed without changing the storage or egress architecture:
/usrtree once (issue Direct sandbox startup fails on merged /usr runner images #22).SANDBOX_BIND_HOST/bind_porthandling so IPv6 listeners do not break on colon splitting (issue Allow for IPv6 clusters #23)./host-packagesmount when available and keeps bakedROOTFS/pkgsas the fallback when the mount is empty or unavailable (partial issue [Feature Request] Add NsJail direct sandbox with baked pkgs #24).Validation
bash tests/start-direct-sandbox.test.shbash -n docker/start-direct-sandbox.sh tests/start-direct-sandbox.test.shnpx --yes bun test api/src/config.test.tsgit diff --checkScope notes
Issue #25 appears to be an injection/lifecycle race in the LibreChat integration: the first execution arrives with zero injected files, while the second has the resource file. It is not reproducibly fixable in this repository from the issue evidence alone.
Issues #15, #21, #11, and #6 require separate upstream design or infrastructure work (virtio-fs/libkrun descriptor lifecycle, Flux-compatible chart sourcing, object-storage migration, and a policy-controlled egress gateway respectively).
References: #22, #23, #24, #25, #15, #21, #11, #6